Skip to content

Conversation

@rowanG077
Copy link
Member

Basically the implementation @christiaanb gave in #3129 with an added test in the testsuite.

Fixes #3129

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

@rowanG077 rowanG077 linked an issue Jan 29, 2026 that may be closed by this pull request
@rowanG077 rowanG077 requested a review from christiaanb January 29, 2026 17:22
@rowanG077 rowanG077 force-pushed the 3129-clash-takes-a-long-time-to-compile-a-recursively-defined-constant branch from 005a66e to 27fe4eb Compare January 29, 2026 17:54
@rowanG077 rowanG077 requested a review from leonschoorl January 30, 2026 08:02
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another day another hack..

Comment on lines +295 to +303
e2Red <- case collectArgs e2 of
(Prim p0, _) -> whnfRW False ctx e2 $ \_ctx1 e2Red -> case e2Red of
(collectArgs -> (Prim p1, _)) | primName p0 == primName p1 -> return e2
_ -> changed e2Red
_ -> return e2
specialize ctx (App e1 e2Red)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should have a comment pointing to the bug report. I know that we can do code archaeology, but given that this is such a hack I think it deserves one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also done

@@ -0,0 +1 @@
FIXED: Reduce constants to NF before specialisation [#3129](https://github.com/clash-lang/clash-compiler/issues/3129)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FIXED: Reduce constants to NF before specialisation [#3129](https://github.com/clash-lang/clash-compiler/issues/3129)
FIXED: Reduce constants to NF before specialization [#3129](https://github.com/clash-lang/clash-compiler/issues/3129)

We settled on 🇺🇸 English

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@rowanG077 rowanG077 force-pushed the 3129-clash-takes-a-long-time-to-compile-a-recursively-defined-constant branch from 27fe4eb to 00e3653 Compare February 8, 2026 21:06
@martijnbastiaan
Copy link
Member

I think you might have amended without mentioning files @rowanG077?

@rowanG077 rowanG077 force-pushed the 3129-clash-takes-a-long-time-to-compile-a-recursively-defined-constant branch from 00e3653 to 9f275d9 Compare February 8, 2026 21:40
@rowanG077
Copy link
Member Author

Ugggh that's what you get when you are too long in front of the computer. It's fixed now.

@rowanG077 rowanG077 merged commit 2ad5b61 into master Feb 9, 2026
41 of 47 checks passed
@rowanG077 rowanG077 deleted the 3129-clash-takes-a-long-time-to-compile-a-recursively-defined-constant branch February 9, 2026 19:45
mergify bot pushed a commit that referenced this pull request Feb 9, 2026
…3130)

(cherry picked from commit 2ad5b61)

# Conflicts:
#	clash-lib/src/Clash/Normalize/Transformations/Specialize.hs
rowanG077 added a commit that referenced this pull request Feb 10, 2026
…3130)

(cherry picked from commit 2ad5b61)

# Conflicts:
#	clash-lib/src/Clash/Normalize/Transformations/Specialize.hs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clash takes a long time to compile a recursively defined constant.

2 participants